Add a "severity" for tests (#1005) #1410
Conversation
beckjake
force-pushed
the
feature/test-severity
branch
4 times, most recently
from
f8633e9
to
88edf14
Compare
There was a problem hiding this comment.
This is really cool - I like the work that you did around TestBuilders and the code works super well.
In playing around with this, I think I have some reservations about parsing the severity out of the test name. While it's really slick for string-type tests, I don't think it holds up for dict-type tests. Eg:
version: 2
models:
- name: my_model
columns:
- name: id
tests:
- unique, severity=ERROR
- relationships, severity=WARN:
from: id
to: ref('other_model')
field: name
The unique example is great and intuitive, but the relationships example feels pretty strange to me. Having played around with this for a few minutes, I think I'd prefer supplying the severity config as an argument to the test, eg:
models:
- name: my_model
columns:
- name: id
tests:
- unique:
severity: ERROR
- relationships:
from: id
to: ref('other_model')
field: name
severity: WARN
This will definitely mask any macro args named severity, which is unpleasant, but I think we could work around it by supporting args like __severity__, or dbt.severity, or similar if need be.
The - unique, severity=ERROR case is super nice, and a lot more pleasant than adding an arg to the unique test config. I don't think it would make sense to support both syntaxes, but I just want to make it clear that I need to sleep on this one.
Let's chat about what our options are here
core/dbt/parser/schemas.py
Outdated
| return modifiers | ||
|
|
||
| for entry in modifiers_value.split(','): | ||
| if '=' not in entry: |
There was a problem hiding this comment.
I don't think this works as expected, the following spec compiles and runs whereas I would expect to see a compiler error:
models:
- name: my_model
columns:
- name: id
tests:
- unique, invalid
8ca0d15
to
0ce2192
Compare
beckjake
force-pushed
the
feature/test-severity
branch
from
0ce2192
to
9c52c6f
Compare
A small refactor to make test parsing easier to modify add concept of test modifier kwargs, pass them through to config plug the severity setting into test result handling Update existing tests Add integration tests severity settings for data tests, too
beckjake
force-pushed
the
feature/test-severity
branch
from
9c52c6f
to
abcbaca
Compare
Fixes #1005
Add an optional severity config for tests that defaults to
ERROR.A test with severity=WARN that fails looks like this:
Those trailing
WARN 2s are yellow, like in source freshness.A test with severity=WARN that fails with the strict flag set looks like this:
Those trailing
FAIL 2s are red.I also refactored some of the schema test parsing code and made a
TestBuilderclass to encapsulate some of the kind of hairy logic that's currently involved in the source/ref test split. I think that came out super nicely.I think this is mostly done, just want to trigger Windows tests to make sure everything went ok there.